Fixed some configs not being synced to clients from server #116
Conversation
Send `DynamicMoldUnits`, `CastToolDurabilityPenalty`, and `DurabilityPenaltyPerRepair` so that tooltips and descriptions work as intended when the config has been modified
📝 WalkthroughWalkthroughThe ConfigLoader is refactored to implement a multi-stage lifecycle pattern, replacing the single Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @SmithingPlus/Config/ConfigLoader.cs:
- Around line 68-73: ReadConfigFromServer currently reads only three of the five
values that SendConfigToClient writes; update ReadConfigFromServer to also read
and assign Config.CanRepairForlornHopeEstoc and Config.WorkableBits from
api.World.Config (use the same keys used in SendConfigToClient, e.g.,
"SmithingPlus_CanRepairForlornHopeEstoc" and "SmithingPlus_WorkableBits") so the
static Config stays in sync with the server; if WorkableBits is intended to be
computed on demand instead, then compute it where needed and still read
CanRepairForlornHopeEstoc here to preserve consistency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SmithingPlus/Config/ConfigLoader.cs
🧰 Additional context used
🧬 Code graph analysis (1)
SmithingPlus/Config/ConfigLoader.cs (1)
SmithingPlus/Config/ServerConfig.cs (1)
ServerConfig(7-53)
🔇 Additional comments (8)
SmithingPlus/Config/ConfigLoader.cs (8)
3-3: LGTM! Required imports for side-specific lifecycle methods.The new imports support the
ICoreClientAPIandICoreServerAPIparameters used in the lifecycle methods.Also applies to: 5-5
20-23: LGTM! Proper server-side initialization.The
StartServerSideoverride correctly callsSendConfigToClientto synchronize config values to connected clients via world config.
30-33: LGTM! Proper early initialization.The
StartPreoverride correctly callsLoadServerConfigto load configuration before side-specific initialization occurs.
35-56: Good refactoring with config verification.The extraction of
LoadServerConfigproperly separates config loading logic, and the addition ofVerifyConfig()(line 47) ensures config validation before persistence.Minor note: The method name
LoadServerConfigis slightly misleading since it runs on both client and server viaStartPre, but it's acceptable since it loads theServerConfigtype.
62-65: PR objective successfully implemented.The three config values specified in the PR objectives (
DynamicMoldUnits,CastToolDurabilityPenalty,DurabilityPenaltyPerRepair) are now correctly transmitted to clients via world config synchronization.
58-66: Config values correctly written to world config.The method properly sets config values in
api.World.Config, which will be synchronized to connected clients by the game engine.Note: Five values are sent but only three are read back in
ReadConfigFromServer. See comment on lines 68-73 for details.
75-87: LGTM! Proper config validation.The
VerifyConfigmethod appropriately validates config values, warns about potential issues, and clampsVoxelsPerBitto the valid range [2, 3].
25-28: No timing concern here. Vintage Story's engine automatically synchronizesWorld.Configfrom server to client beforeStartClientSideis called. Your implementation correctly reads the synchronized values without needing custom network channels or delayed callbacks.
| public void ReadConfigFromServer(ICoreAPI api) | ||
| { | ||
| Config.DynamicMoldUnits = api.World.Config.GetBool("SmithingPlus_DynamicMoldUnits"); | ||
| Config.CastToolDurabilityPenalty = api.World.Config.GetFloat("SmithingPlus_CastToolDurabilityPenalty"); | ||
| Config.DurabilityPenaltyPerRepair = api.World.Config.GetFloat("SmithingPlus_DurabilityPenaltyPerRepair"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the ConfigLoader.cs file to see SendConfigToClient method and get full context
find . -name "ConfigLoader.cs" -type fRepository: jayugg/SmithingPlus
Length of output: 100
🏁 Script executed:
# Look for the file and get its size
fd ConfigLoader.csRepository: jayugg/SmithingPlus
Length of output: 98
🏁 Script executed:
# Examine the ConfigLoader.cs file
cat -n SmithingPlus/Config/ConfigLoader.cs | head -100Repository: jayugg/SmithingPlus
Length of output: 3865
🏁 Script executed:
# Search for SendConfigToClient method to see what values are sent
rg -n "SendConfigToClient" --type=cs -A 10 SmithingPlus/Config/ConfigLoader.csRepository: jayugg/SmithingPlus
Length of output: 1035
🏁 Script executed:
# Search for usage of ConfigLoader.Config properties on client side
rg -n "ConfigLoader\.Config\." --type=csRepository: jayugg/SmithingPlus
Length of output: 45
🏁 Script executed:
# Also search for direct Config property access patterns
rg -n "Config\.(CanRepairForlornHopeEstoc|SmithWithBits|EnableToolRecovery|DynamicMoldUnits|CastToolDurabilityPenalty|DurabilityPenaltyPerRepair|WorkableBits)" --type=csRepository: jayugg/SmithingPlus
Length of output: 2363
🏁 Script executed:
# Check if CanRepairForlornHopeEstoc is used anywhere on client side
rg -n "CanRepairForlornHopeEstoc" --type=csRepository: jayugg/SmithingPlus
Length of output: 314
🏁 Script executed:
# Search for how WorkableBits is used
rg -n "WorkableBits" --type=cs -B 2 -A 2Repository: jayugg/SmithingPlus
Length of output: 588
Incomplete synchronization: read back all sent config values for consistency.
SendConfigToClient sends 5 values to world config (lines 60-65), but ReadConfigFromServer only reads back 3 of them:
- ✅ Read back:
DynamicMoldUnits,CastToolDurabilityPenalty,DurabilityPenaltyPerRepair - ❌ Not read back:
CanRepairForlornHopeEstoc(line 60),WorkableBits(line 61)
While CanRepairForlornHopeEstoc and WorkableBits are not currently used on the client side, the missing synchronization creates inconsistency and violates the pattern established by the other config values. Since the server explicitly sends these values to the world config, they should be read back on the client to ensure the static Config object stays in sync with the server.
Add the missing properties to ReadConfigFromServer:
Suggested fix
public void ReadConfigFromServer(ICoreAPI api)
{
+ Config.CanRepairForlornHopeEstoc = api.World.Config.GetBool("SmithingPlus_CanRepairForlornHopeEstoc");
Config.DynamicMoldUnits = api.World.Config.GetBool("SmithingPlus_DynamicMoldUnits");
Config.CastToolDurabilityPenalty = api.World.Config.GetFloat("SmithingPlus_CastToolDurabilityPenalty");
Config.DurabilityPenaltyPerRepair = api.World.Config.GetFloat("SmithingPlus_DurabilityPenaltyPerRepair");
}Note: WorkableBits is a computed value derived from SmithWithBits or EnableToolRecovery and does not need to be stored as a config property; it can be computed on demand when needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void ReadConfigFromServer(ICoreAPI api) | |
| { | |
| Config.DynamicMoldUnits = api.World.Config.GetBool("SmithingPlus_DynamicMoldUnits"); | |
| Config.CastToolDurabilityPenalty = api.World.Config.GetFloat("SmithingPlus_CastToolDurabilityPenalty"); | |
| Config.DurabilityPenaltyPerRepair = api.World.Config.GetFloat("SmithingPlus_DurabilityPenaltyPerRepair"); | |
| } | |
| public void ReadConfigFromServer(ICoreAPI api) | |
| { | |
| Config.CanRepairForlornHopeEstoc = api.World.Config.GetBool("SmithingPlus_CanRepairForlornHopeEstoc"); | |
| Config.DynamicMoldUnits = api.World.Config.GetBool("SmithingPlus_DynamicMoldUnits"); | |
| Config.CastToolDurabilityPenalty = api.World.Config.GetFloat("SmithingPlus_CastToolDurabilityPenalty"); | |
| Config.DurabilityPenaltyPerRepair = api.World.Config.GetFloat("SmithingPlus_DurabilityPenaltyPerRepair"); | |
| } |
🤖 Prompt for AI Agents
In @SmithingPlus/Config/ConfigLoader.cs around lines 68 - 73,
ReadConfigFromServer currently reads only three of the five values that
SendConfigToClient writes; update ReadConfigFromServer to also read and assign
Config.CanRepairForlornHopeEstoc and Config.WorkableBits from api.World.Config
(use the same keys used in SendConfigToClient, e.g.,
"SmithingPlus_CanRepairForlornHopeEstoc" and "SmithingPlus_WorkableBits") so the
static Config stays in sync with the server; if WorkableBits is intended to be
computed on demand instead, then compute it where needed and still read
CanRepairForlornHopeEstoc here to preserve consistency.
1 participant
Send
DynamicMoldUnits,CastToolDurabilityPenalty, andDurabilityPenaltyPerRepairto clients from the server so that tooltips and descriptions work as intended when the config has been modified.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.